Remove gempyor.seeding Dependence On gempyor.model_info.ModelInfo
#422
+225
−122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Added explicit exports, * Reordered imports, * Added general structure comments, and * Refactored all lines to be less than 92 characters.
Remvoed the `modinf` arg from the `_DataFrame2NumbaDict` internal utility and replaced it with the individual attributes from the `modinf` object that are needed.
Untangled the `Seeding` class from the `ModelInfo` class by adding a method to `ModelInfo` that interfaces with the `Seeding` class.
Updated remaining references in inactive code, notebooks and the `gempyor.dev` module.
TimothyWillard
added
enhancement
TimothyWillard
added
the
next release
TimothyWillard
requested review from
pearsonca,
jcblemai,
fang19911030 and
emprzy
pearsonca
reviewed
Jan 7, 2025
| @@ -164,7 +164,7 @@ def get_static_arguments(modinf: model_info.ModelInfo): | |||
| ) | |||
|
|
|||
| initial_conditions = modinf.initial_conditions.get_from_config(sim_id=0, modinf=modinf) | |||
| seeding_data, seeding_amounts = modinf.seeding.get_from_config(sim_id=0, modinf=modinf) | |||
| seeding_data, seeding_amounts = modinf.get_seeding_data(0) | |||
There was a problem hiding this comment.
seems preferrable to leave the explicity sim_id=0 style
| run_id=run_id, | ||
| prefix=prefix, | ||
| index=sim_id + self.first_sim_index - 1, | ||
| return self.path_prefix / file_paths.create_file_name( |
There was a problem hiding this comment.
again, let's preserve the argument= style here, to save ourselves future heartache
| seeding_data, seeding_amounts = modinf.seeding.get_from_file( | ||
| sim_id2load, modinf=modinf | ||
| ) | ||
| seeding_data, seeding_amounts = modinf.get_seeding_data(sim_id2load) |
There was a problem hiding this comment.
not pulling this out of the if-else, like in inference.py?
emprzy
approved these changes
Jan 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes.
This pull request untangles
gempyor.seeding.Seedingfromgempyor.model_info.ModelInfoby requiring explicit values be passed togempyor.seeding.Seeding. This eases the unit testability of thegempyor.seedingmodule. Also adds theModelInfo.get_seeding_datamethod which replaces prior calls to theseedinginstance from theModelnfoinstance with amodinfargument.Does this pull request make any user interface changes? If so please describe.
The user interface changes are:
gempyor.model_info.ModelInfo.get_seeding_datamethod,gempyor.seeding.Seeding.get_from_config/filemethods, andgempyor.seeding.Seeding.get_from_filein favor ofgempyor.seeding.Seeding.get_from_config.Those are reflected in updates to the documentation in the docstrings of these methods.
What does your pull request address? Tag relevant issues.
This pull request addresses part of GH-397.